-
Notifications
You must be signed in to change notification settings - Fork 27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fixed circular ref false positive during property resolution with mul… #269
Conversation
@@ -171,8 +181,7 @@ protected Object resolveReferenceInDepth(Reference reference, Map<String, Object | |||
currentProperty = resolveKeyInIterable(keyPart, (Collection<?>) currentProperty, deepReferenceKey); | |||
keyPart = ""; | |||
} else if (currentProperty instanceof Map) { | |||
@SuppressWarnings("unchecked") | |||
Object subProperty = MapUtils.getObject((Map<Object, ?>) currentProperty, keyPart); | |||
@SuppressWarnings("unchecked") Object subProperty = MapUtils.getObject((Map<Object, ?>) currentProperty, keyPart); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the rest of the code @SuppressWarnings("unchecked") is on a separate row, please align
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
boolean resolutionContextWasCreated = false; | ||
HashSet<String> contextKeysBackup = null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't it better to use Collections.emptySet()
for collections instead of null.
@@ -171,8 +181,7 @@ protected Object resolveReferenceInDepth(Reference reference, Map<String, Object | |||
currentProperty = resolveKeyInIterable(keyPart, (Collection<?>) currentProperty, deepReferenceKey); | |||
keyPart = ""; | |||
} else if (currentProperty instanceof Map) { | |||
@SuppressWarnings("unchecked") | |||
Object subProperty = MapUtils.getObject((Map<Object, ?>) currentProperty, keyPart); | |||
@SuppressWarnings("unchecked") Object subProperty = MapUtils.getObject((Map<Object, ?>) currentProperty, keyPart); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
void testResolve(String parameterName, String parameterValue, Expectation expectation) { | ||
PropertiesResolver testResolver = new PropertiesResolver(null, irrelevant -> replacementValues, ReferencePattern.PLACEHOLDER, | ||
"test-", false, Collections.emptySet()); | ||
tester.test(() -> testResolver.visit(parameterName, parameterValue), expectation); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why invoked method was changed from resolveReferenceInDepth
to visit
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the previous method only tested a small portion of the code and couldn't resolve variables in depth (variables containing refs to other refs)
b23c3d3
to
bf25337
Compare
Quality Gate passedIssues Measures |
…tiple references
LMCROSSITXSADEPLOY-2936
Using multiple references linking to same variable was incorrectly detected as a circular reference - Fixed ref key context to reset on each subsequent ref.
Fixed error message in Tester class